THRIFT-6073: Allow injecting external SSL_CTX into C++ SSLContext by hongzhi-gao · Pull Request #3606 · apache/thrift

hongzhi-gao · 2026-06-26T09:26:50Z

Summary

Add a backend-neutral extension point to the C++ TSSLSocket stack: applications can inject a pre-configured OpenSSL SSL_CTX through the existing SSLContextFactory / TSSLSocketFactory hook.

This makes it easier for applications to supply a fully configured TLS context—protocol options, cipher suites, certificate loading, and other OpenSSL settings—in application code, without patching libthrift or replacing the transport layer.

Background

Thrift C++ already provides SSL/TLS through TSSLSocket / TSSLSocketFactory, backed by OpenSSL. The default factory covers the common case: a single certificate/key pair via loadCertificate() / loadPrivateKey(), and protocol selection via SSLProtocol.

TSSLSocketFactory already accepts a custom SSLContextFactory, but SSLContext could previously only be created from SSLProtocol. Applications that need a context configured outside Thrift's helpers—multi-step cert setup, non-default methods, or options not exposed by TSSLSocketFactory—had to patch Thrift locally.

This PR adds a small wrapper constructor on SSLContext while keeping the default OpenSSL build unchanged.

What changed

SSLContext: explicit SSLContext(SSL_CTX* ctx, bool takeOwnership = true) — wrap an application-owned context; destructor respects ownership.
lib/cpp/README.md: document the injection pattern.
SecurityTest: wrapped-context and null-validation coverage.

Removed from the earlier draft: WITH_TONGSUO, SSLProtocol::NTLS, loadSign* / loadEnc*, vendored test keys, and the optional local NTLS integration test.

Usage

Standard TLS (unchanged):

TSSLSocketFactory factory;  // default SSLTLS
factory.loadCertificate("server.crt");
factory.loadPrivateKey("server.key");

Injected SSL_CTX (application owns OpenSSL configuration):

#include <thrift/transport/TSSLSocket.h>

auto factory = std::make_shared<TSSLSocketFactory>([]() {
  SSL_CTX* ctx = SSL_CTX_new(TLS_method());
  SSL_CTX_set_mode(ctx, SSL_MODE_AUTO_RETRY);
  // Configure ctx as needed: ciphers, certificates, verify mode, etc.
  return std::make_shared<apache::thrift::transport::SSLContext>(ctx, true);
});
factory->loadTrustedCertificates("ca-bundle.crt");
factory->authenticate(true);
factory->server(true);
auto socket = factory->createSocket("127.0.0.1", port);

Link your application against whichever OpenSSL-compatible TLS library you choose; libthrift continues to default to system OpenSSL at build time.

Licensing

Thrift introduces no new build-time dependency. Nothing is vendored; no LICENSE / NOTICE update is required.

Test plan

Standard OpenSSL build: ctest -R SecurityTest (wrapped_ssl_context, wrapped_ssl_context_null)

Enable building libthrift against Tongsuo via -DWITH_TONGSUO and expose NTLS dual-certificate APIs on TSSLSocketFactory for TLCP workloads.

Jens-G · 2026-06-26T21:35:18Z

Code review

Found 2 issues:

All 8 new load* methods call requireNtlsSupport() before the null-pointer argument check, reversing the established contract of every existing load* method in the file (loadCertificate, loadPrivateKey, etc. all validate arguments first). On non-NTLS builds, callers that pass null arguments receive TSSLException("NTLS is not available") instead of TTransportException(BAD_ARGS), making errors undiagnosable.

thrift/lib/cpp/src/thrift/transport/TSSLSocket.cpp

Lines 1110 to 1115 in 0d5e69a

    
           void TSSLSocketFactory::loadSignCertificate(const char* path, const char* format) { 
        
             requireNtlsSupport(); 
        
             if (path == nullptr || format == nullptr) { 
        
               throw TTransportException(TTransportException::BAD_ARGS, 
        
                                         "loadSignCertificate: either <path> or <format> is nullptr"); 
        
             }

Tongsuo is introduced as a new optional build dependency but the PR description contains no mention of license verification against the ASF Category A/X list, as required by AGENTS.md §1: "Before introducing any dependency … verify its license is compatible with Apache 2.0 … If in doubt … add it and flag it in the PR description for committer review."

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

HTHou · 2026-06-27T13:40:05Z

  TLSv1_0 = 3,  // Supports TLSv1_0 or later.
  TLSv1_1 = 4,  // Supports TLSv1_1 or later.
  TLSv1_2 = 5,  // Supports TLSv1_2 or later.
+  NTLS    = 6,  // Tongsuo NTLS/TLCP; requires dual signing and encryption certificates.


Should 6 be TLSv1_3?

NTLS = 6 was removed with the provider-specific APIs.

HTHou · 2026-06-27T13:49:42Z

Thanks for putting this together. Could we consider narrowing the upstream change to a generic TLS-provider compatibility layer, rather than making Tongsuo/NTLS a first-class Thrift backend?

The current patch adds Tongsuo-specific build plumbing, SSLProtocol::NTLS, and loadSign* / loadEnc* APIs to TSSLSocketFactory. That makes the Tongsuo/TLCP surface part of Thrift's public C++ API/ABI and ongoing build/test matrix.

An alternative might be to expose a small backend-neutral extension point around SSLContext / SSL_CTX creation and configuration, for example allowing applications to provide an already-created SSLContext/SSL_CTX, or a protected/public hook that creates and configures the context. Then downstream applications that need Tongsuo can link against it themselves and apply the NTLS/TLCP dual-certificate setup in their own factory/subclass, while Thrift only maintains the generic OpenSSL-compatible transport layer.

That would still let users avoid carrying a private transport fork, but keeps the provider-specific dependency and policy choices outside the core Thrift tree. Would you be open to reshaping the patch in that direction?

hongzhi-gao · 2026-06-30T07:16:43Z

Thanks for putting this together. Could we consider narrowing the upstream change to a generic TLS-provider compatibility layer, rather than making Tongsuo/NTLS a first-class Thrift backend?

The current patch adds Tongsuo-specific build plumbing, SSLProtocol::NTLS, and loadSign* / loadEnc* APIs to TSSLSocketFactory. That makes the Tongsuo/TLCP surface part of Thrift's public C++ API/ABI and ongoing build/test matrix.

An alternative might be to expose a small backend-neutral extension point around SSLContext / SSL_CTX creation and configuration, for example allowing applications to provide an already-created SSLContext/SSL_CTX, or a protected/public hook that creates and configures the context. Then downstream applications that need Tongsuo can link against it themselves and apply the NTLS/TLCP dual-certificate setup in their own factory/subclass, while Thrift only maintains the generic OpenSSL-compatible transport layer.

That would still let users avoid carrying a private transport fork, but keeps the provider-specific dependency and policy choices outside the core Thrift tree. Would you be open to reshaping the patch in that direction?

Fixed by removing the Tongsuo-specific loadSign* / loadEnc* APIs entirely. The reshaped patch no longer adds those methods.
Added a Licensing section: Thrift still depends only on OpenSSL by default; Tongsuo is an application-side choice (Apache 2.0, ASF Category A) and is not vendored or added to Thrift's build.

hongzhi-gao · 2026-06-30T07:17:37Z

Thanks for putting this together. Could we consider narrowing the upstream change to a generic TLS-provider compatibility layer, rather than making Tongsuo/NTLS a first-class Thrift backend?

The current patch adds Tongsuo-specific build plumbing, SSLProtocol::NTLS, and loadSign* / loadEnc* APIs to TSSLSocketFactory. That makes the Tongsuo/TLCP surface part of Thrift's public C++ API/ABI and ongoing build/test matrix.

An alternative might be to expose a small backend-neutral extension point around SSLContext / SSL_CTX creation and configuration, for example allowing applications to provide an already-created SSLContext/SSL_CTX, or a protected/public hook that creates and configures the context. Then downstream applications that need Tongsuo can link against it themselves and apply the NTLS/TLCP dual-certificate setup in their own factory/subclass, while Thrift only maintains the generic OpenSSL-compatible transport layer.

That would still let users avoid carrying a private transport fork, but keeps the provider-specific dependency and policy choices outside the core Thrift tree. Would you be open to reshaping the patch in that direction?

Done — the patch is reshaped to a backend-neutral SSLContext wrapper around an application-provided SSL_CTX, using the existing SSLContextFactory hook. Tongsuo/NTLS dual-cert setup lives in application (or local test) code; Thrift core stays OpenSSL-compatible and provider-agnostic.

Replace Tongsuo-specific build options, NTLS protocol enum, and dual-certificate factory APIs with SSLContext(SSL_CTX*, bool takeOwnership) so applications can inject a pre-configured OpenSSL-compatible context through SSLContextFactory.

Prevent implicit conversion from SSL_CTX* to avoid accidental ownership mistakes.

Clarify that CTest expects a sibling Tongsuo source checkout for SM2 fixtures and that shared TLS libraries from a build tree need LD_LIBRARY_PATH.

Keep the PR focused on backend-neutral SSLContext injection; SecurityTest covers the wrapped-context API.

Add Tongsuo NTLS support to C++ TSSLSocket

0d5e69a

Enable building libthrift against Tongsuo via -DWITH_TONGSUO and expose NTLS dual-certificate APIs on TSSLSocketFactory for TLCP workloads.

hongzhi-gao requested review from Jens-G, emmenlau and mhlakhani as code owners June 26, 2026 09:26

mergeable Bot added c++ Pull requests that update C++ code testsuite build and general CI cmake, automake and build system changes labels Jun 26, 2026

hongzhi-gao changed the title ~~Add Tongsuo NTLS support to C++ TSSLSocket~~ Add optional TLCP/NTLS support to C++ TSSLSocket via Tongsuo Jun 26, 2026

hongzhi-gao changed the title ~~Add optional TLCP/NTLS support to C++ TSSLSocket via Tongsuo~~ THRIFT-6073: Add optional TLCP/NTLS support to C++ TSSLSocket via Tongsuo Jun 26, 2026

HTHou reviewed Jun 27, 2026

View reviewed changes

hongzhi-gao added 4 commits June 30, 2026 15:33

Mark injected SSLContext constructor explicit.

8ffcd87

Prevent implicit conversion from SSL_CTX* to avoid accidental ownership mistakes.

Document NTLS local test cert path and LD_LIBRARY_PATH notes.

87ebd57

Clarify that CTest expects a sibling Tongsuo source checkout for SM2 fixtures and that shared TLS libraries from a build tree need LD_LIBRARY_PATH.

Remove optional NTLS local test and related CMake/docs.

0870077

Keep the PR focused on backend-neutral SSLContext injection; SecurityTest covers the wrapped-context API.

HTHou approved these changes Jun 30, 2026

View reviewed changes

hongzhi-gao changed the title ~~THRIFT-6073: Add optional TLCP/NTLS support to C++ TSSLSocket via Tongsuo~~ THRIFT-6073: Support Injecting Custom SSL_CTX into TSSLSocket Jun 30, 2026

hongzhi-gao changed the title ~~THRIFT-6073: Support Injecting Custom SSL_CTX into TSSLSocket~~ THRIFT-6073: Allow injecting external SSL_CTX into C++ SSLContext Jun 30, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

THRIFT-6073: Allow injecting external SSL_CTX into C++ SSLContext#3606

THRIFT-6073: Allow injecting external SSL_CTX into C++ SSLContext#3606
hongzhi-gao wants to merge 5 commits into
apache:masterfrom
hongzhi-gao:feature/cpp-tongsuo-ntls

hongzhi-gao commented Jun 26, 2026 •

edited

Loading

Uh oh!

Jens-G commented Jun 26, 2026 •

edited

Loading

Uh oh!

HTHou Jun 27, 2026 •

edited

Loading

Uh oh!

hongzhi-gao Jun 30, 2026

Uh oh!

HTHou commented Jun 27, 2026

Uh oh!

hongzhi-gao commented Jun 30, 2026

Uh oh!

hongzhi-gao commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

hongzhi-gao commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Background

What changed

Usage

Licensing

Test plan

Uh oh!

Jens-G commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review

Uh oh!

HTHou Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hongzhi-gao Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

HTHou commented Jun 27, 2026

Uh oh!

hongzhi-gao commented Jun 30, 2026

Uh oh!

hongzhi-gao commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hongzhi-gao commented Jun 26, 2026 •

edited

Loading

Jens-G commented Jun 26, 2026 •

edited

Loading

HTHou Jun 27, 2026 •

edited

Loading